Skip to content

[16.0][OU-ADD] hr_expense: Migration to 16.0 - #4156

Merged
pedrobaeza merged 2 commits into
OCA:16.0from
Tecnativa:16.0-mig-hr_expense
Mar 8, 2024
Merged

[16.0][OU-ADD] hr_expense: Migration to 16.0#4156
pedrobaeza merged 2 commits into
OCA:16.0from
Tecnativa:16.0-mig-hr_expense

Conversation

@victoralmau

@victoralmau victoralmau commented Oct 11, 2023

Copy link
Copy Markdown
Member

Migration to 16.0

Depends on:

@Tecnativa TT43223

@legalsylvain

legalsylvain commented Oct 11, 2023

Copy link
Copy Markdown
Contributor

/ocabot migration hr_expense

Depends on :

@marielejeune marielejeune left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for the migration script. Here is an error that I got.

Comment thread openupgrade_scripts/scripts/hr_expense/16.0.2.0/pre-migration.py Outdated

@marielejeune marielejeune left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more comments

Comment thread openupgrade_scripts/scripts/hr_expense/16.0.2.0/post-migration.py Outdated
Comment thread openupgrade_scripts/scripts/hr_expense/16.0.2.0/post-migration.py Outdated
Comment thread openupgrade_scripts/scripts/hr_expense/16.0.2.0/post-migration.py Outdated
Comment thread openupgrade_scripts/scripts/hr_expense/16.0.2.0/post-migration.py Outdated

@remi-filament remi-filament left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @victoralmau , thank you for this PR.
I have tested your module when migrating our instance, and I face an issue when trying to reconcile payment on account move (state and payment_state are not updated to paid on hr.expense.sheet).
After some digging, I realized that from v16.0 onwards, account_move related to hr_expense_sheet are created with move_type = "in_invoice" (when there was no move_type before so it ended up with default "entry") as per https://github.com/odoo/odoo/blob/87f4667d81f7d8a40dcf225f3daf1a9e2795680d/addons/hr_expense/models/hr_expense.py#L1297

Then in _compute_payment_state from account_move it will check if account.move is of type invoice (with is_invoice function here : https://github.com/odoo/odoo/blob/87f4667d81f7d8a40dcf225f3daf1a9e2795680d/addons/account/models/account_move.py#L926 and if move_type is "entry" it would ends up with payment_state = "not_paid".

Therefore, I think that this migration script needs to update move_type for all moves with an expense_sheet_id to "in_invoice"

@pedrobaeza
pedrobaeza force-pushed the 16.0-mig-hr_expense branch 2 times, most recently from 901e37c to 1c90b94 Compare March 8, 2024 20:05

@pedrobaeza pedrobaeza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the comments attended including @remi-filament's one, plus the final fixes + improvements I have done reviewing the PR

@pedrobaeza
pedrobaeza force-pushed the 16.0-mig-hr_expense branch from 1c90b94 to 55df4ef Compare March 8, 2024 20:14
@pedrobaeza
pedrobaeza force-pushed the 16.0-mig-hr_expense branch from 55df4ef to 07f62e8 Compare March 8, 2024 20:52
@pedrobaeza
pedrobaeza merged commit 5a230b1 into OCA:16.0 Mar 8, 2024
@pedrobaeza
pedrobaeza deleted the 16.0-mig-hr_expense branch March 8, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants